Fix connection direction #7076

youngbupark · 2024-01-25T21:02:00Z

Description

This is to fix appgraph connection direction.

Type of change

This pull request fixes a bug in Radius and has an approved issue (issue link required).

Fixes: #7038

Signed-off-by: Young Bu Park <[email protected]>

youngbupark · 2024-01-25T21:03:46Z

pkg/corerp/frontend/controller/applications/testdata/graph-app-directroute-out.json

@@ -2,7 +2,7 @@
    {


@nithyatsu @rynowak Please carefully review these test cases.

This one is right.

test/functional/shared/resources/testdata/corerp-resources-application-graph-out.json

radius-functional-tests · 2024-01-25T22:17:47Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	youngbupark/radius
Commit ref	`a56ecf5`
Unique ID	636a631d8b
Image tag	pr-636a631d8b

Click here to see the list of tools in the current test run

gotestsum 1.10.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/functional/shared/recipes/<name>:pr-636a631d8b
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-636a631d8b
controller test image location: ghcr.io/radius-project/dev/controller:pr-636a631d8b
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-636a631d8b
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting msgrp functional tests...
⌛ Starting samples functional tests...
⌛ Starting ucp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting shared functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting datastoresrp functional tests...
✅ msgrp functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ samples functional tests succeeded
✅ kubernetes functional tests succeeded
✅ ucp functional tests succeeded
✅ daprrp functional tests succeeded
❌ shared functional test failed. Please check the logs for more details

youngbupark · 2024-01-25T22:18:59Z

pkg/corerp/frontend/controller/applications/testdata/graph-app-httproute-out.json

-                "id": "/planes/radius/local/resourcegroups/default/providers/Applications.Core/containers/sql-ctnr"
-            }
-        ],
+        "connections": [],


This is correct because no other container resources refers to http route.

I'm not really going to comment on the correctness of the HTTP route scenario. It would take extra work specific to HTTP Routes to get a useful output for this case.

We shouldn't fix it, we should complete the removal of the HTTP Route type.

If we want to make it correct, then httproute should have outbound and inbound both.

youngbupark · 2024-01-25T22:20:03Z

pkg/corerp/frontend/controller/applications/graph_util.go

-					ID:        &id,
-					Direction: &dir, //Direction is set with respect to Resource defining this connection
+					ID:        to.Ptr(id),
+					Direction: to.Ptr(corerpv20231001preview.DirectionInbound), //Direction is set with respect to Resource defining this connection


Inbound connection should be Inbound not Outbound.

youngbupark · 2024-01-25T22:20:36Z

pkg/corerp/frontend/controller/applications/graph_util.go

@@ -493,7 +492,7 @@ func connectionsFromAPIData(resource generated.GenericResource, allResources []g
 	// If we encounter an error processing this data, just skip "invalid" connection entry.
 	entries := []*corerpv20231001preview.ApplicationGraphConnection{}
 	for _, connection := range connections {
-		dir := corerpv20231001preview.DirectionInbound
+		dir := corerpv20231001preview.DirectionOutbound


This is the root cause of this bug.

youngbupark · 2024-01-25T22:21:30Z

pkg/corerp/frontend/controller/applications/graph_util_test.go

@@ -140,6 +142,13 @@ func Test_computeGraph(t *testing.T) {
 			envResourceDataFile: "",
 			expectedDataFile:    "graph-app-httproute-out.json",
 		},
+		{
+			name:                "using httproute 2",


I am going to change test case name to more meaningful one later.

Signed-off-by: Young Bu Park <[email protected]>

radius-functional-tests · 2024-01-25T22:43:20Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	youngbupark/radius
Commit ref	`7d4e653`
Unique ID	1470c91abd
Image tag	pr-1470c91abd

Click here to see the list of tools in the current test run

gotestsum 1.10.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/functional/shared/recipes/<name>:pr-1470c91abd
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-1470c91abd
controller test image location: ghcr.io/radius-project/dev/controller:pr-1470c91abd
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-1470c91abd
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting datastoresrp functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting samples functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting shared functional tests...
⌛ Starting daprrp functional tests...
✅ datastoresrp functional tests succeeded
✅ msgrp functional tests succeeded
✅ kubernetes functional tests succeeded
✅ ucp functional tests succeeded
✅ daprrp functional tests succeeded
✅ shared functional tests succeeded

# Description This is to fix appgraph connection direction. ## Type of change - This pull request fixes a bug in Radius and has an approved issue (issue link required). Fixes: radius-project#7038 --------- Signed-off-by: Young Bu Park <[email protected]>

# Description This is to fix appgraph connection direction. ## Type of change - This pull request fixes a bug in Radius and has an approved issue (issue link required). Fixes: radius-project#7038 --------- Signed-off-by: Young Bu Park <[email protected]> Signed-off-by: sk593 <[email protected]>

# Description This is to fix appgraph connection direction. ## Type of change - This pull request fixes a bug in Radius and has an approved issue (issue link required). Fixes: radius-project#7038 --------- Signed-off-by: Young Bu Park <[email protected]>

Fix connection direction

a56ecf5

Signed-off-by: Young Bu Park <[email protected]>

youngbupark commented Jan 25, 2024

View reviewed changes

test/functional/shared/resources/testdata/corerp-resources-application-graph-out.json Show resolved Hide resolved

youngbupark temporarily deployed to functional-tests January 25, 2024 22:17 — with GitHub Actions Inactive

youngbupark commented Jan 25, 2024

View reviewed changes

youngbupark added 2 commits January 25, 2024 14:40

refactor functional test code

4cff3b0

Signed-off-by: Young Bu Park <[email protected]>

clean up

7d4e653

Signed-off-by: Young Bu Park <[email protected]>

youngbupark temporarily deployed to functional-tests January 25, 2024 22:42 — with GitHub Actions Inactive

youngbupark changed the title ~~[WIP] Fix connection direction~~ Fix connection direction Jan 25, 2024

youngbupark marked this pull request as ready for review January 25, 2024 23:31

youngbupark requested review from a team as code owners January 25, 2024 23:31

rynowak approved these changes Jan 25, 2024

View reviewed changes

youngbupark merged commit 096e202 into radius-project:main Jan 25, 2024
16 checks passed

youngbupark deleted the youngp/fix-connection branch January 25, 2024 23:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix connection direction #7076

Fix connection direction #7076

youngbupark commented Jan 25, 2024

youngbupark Jan 25, 2024

rynowak Jan 25, 2024

radius-functional-tests bot commented Jan 25, 2024 •

edited

Loading

youngbupark Jan 25, 2024 •

edited

Loading

rynowak Jan 25, 2024 •

edited

Loading

youngbupark Jan 25, 2024

youngbupark Jan 25, 2024

youngbupark Jan 25, 2024

youngbupark Jan 25, 2024

radius-functional-tests bot commented Jan 25, 2024 •

edited

Loading

Fix connection direction #7076

Fix connection direction #7076

Conversation

youngbupark commented Jan 25, 2024

Description

Type of change

youngbupark Jan 25, 2024

Choose a reason for hiding this comment

rynowak Jan 25, 2024

Choose a reason for hiding this comment

radius-functional-tests bot commented Jan 25, 2024 • edited Loading

Radius functional test overview

Test Status

youngbupark Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

rynowak Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

youngbupark Jan 25, 2024

Choose a reason for hiding this comment

youngbupark Jan 25, 2024

Choose a reason for hiding this comment

youngbupark Jan 25, 2024

Choose a reason for hiding this comment

youngbupark Jan 25, 2024

Choose a reason for hiding this comment

radius-functional-tests bot commented Jan 25, 2024 • edited Loading

Radius functional test overview

Test Status

radius-functional-tests bot commented Jan 25, 2024 •

edited

Loading

youngbupark Jan 25, 2024 •

edited

Loading

rynowak Jan 25, 2024 •

edited

Loading

radius-functional-tests bot commented Jan 25, 2024 •

edited

Loading